Skip to content

[Mobile Payments] update tap on mobile designs #8358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 8 commits into from
Jan 9, 2023
Merged

[Mobile Payments] update tap on mobile designs #8358

merged 8 commits into from
Jan 9, 2023

Conversation

@joshheald
Copy link
Contributor

@joshheald joshheald commented Dec 8, 2022 •
edited
Loading

Closes: #8355

Description

In sTAP Away, we're adding support for payments using the built-in card reader. With two routes through the payment screens, it's helpful to the user to be visually clear whether they are using the built in reader, or an external bluetooth reader.

This PR adds new images for the built in reader flow.

Testing instructions

Using an iPhone XS or newer on iOS 16.0 or newer

  1. Navigate to Menu > Settings > Experimental features
  2. Turn on Tap to Pay on iPhone
  3. Navigate to Menu > Payments > Collect payment
  4. Go through the payment flow, and select Card on the payment method screen
  5. When asked for a reader type, tap Tap to Pay on iPhone and go through the Terms of Service Apple ID linking (if you've not done so before)
  6. Tap the card to pay
  7. Observe that the screens are all updated to match the new design

Disconnect the (built in) reader in Manage card reader

Repeat the test with the legacy bluetooth flow, tapping your card on the reader
Observe that the existing designs are still used for the bluetooth reader flow.

Screenshots

tap.on.iphone.new.images.mp4
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@joshheald joshheald added type: enhancement A request for an enhancement. feature: mobile payments Related to mobile payments / card present payments / Woo Payments. labels Dec 8, 2022
@joshheald joshheald added this to the 11.7 milestone Dec 8, 2022
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 8, 2022 •
edited
Loading

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr8358-41d1e1f on your iPhone

If you need access to App Center, please ask a maintainer to add you.

Base automatically changed from issue/8289-show-follow-instructions-during-built-in-collect-payment to trunk December 9, 2022 17:26
@malinajirka malinajirka removed this from the 11.7 milestone Dec 16, 2022
@joshheald joshheald added the status: feature-flagged Behind a feature flag. Milestone is not strongly held. label Jan 5, 2023
@joshheald joshheald marked this pull request as ready for review January 5, 2023 14:21
@joshheald joshheald added this to the 11.9 milestone Jan 5, 2023
@joshheald
Copy link
Contributor Author

joshheald commented Jan 5, 2023

@toupper No hurry, as there's no benefit to getting this in tomorrow's freeze.

@jostnes @lmischner Optional review again: this adds the new images for the built in screens but otherwise makes no change to functionality. It might be easier to review with the next PR if you're busy.

@toupper toupper self-assigned this Jan 5, 2023
@toupper
Copy link
Contributor

toupper commented Jan 5, 2023

Great work @joshheald, the process looks more custom for the built-in card reader. The code looks good to me, however, I found a few issues when testing the flow design in landscape mode:

Misplaced button

Preparing reader screen shows randomly different configurations. Rotate the device multiple times to achieve this:

@joshheald
8355 Hide IPP image and progress views in landscape
6c439ee 
In the past, we hid the image when the IPP payment flow was used in landscape.

With the addition of the progress view in #8079 c: 83f1d4c  we added another place where the image view could be shown or hidden (as the progress view replaces the image.)

That introduced a bug where images would be shown in landscape, breaking the rest of the IPP layout.

This commit moves all the `isHidden` changes to the `resetHeightAndWidth`, which is responsible for whether and which view should be shown.
@joshheald
Copy link
Contributor Author

joshheald commented Jan 6, 2023

Thanks for the review @toupper. All resolved in 6c439ee – the previous implementation (before #8115) hid the image at all times in landscape, and I've moved back to that.

Showing the images in landscape was unintentional, and the interplay of show/hide images with the progress indicator caused the issues you found.

[...] the process looks more custom for the built-in card reader. The code looks good to me, however, I found a few issues when testing the flow design in landscape mode:

Misplaced button

Preparing reader screen shows randomly different configurations. Rotate the device multiple times to achieve this:

Here's a video with the updates:

landscape.IPP.mp4

@jostnes jostnes mentioned this pull request Jan 9, 2023
Closed
@jostnes
Copy link
Contributor

jostnes commented Jan 9, 2023 •
edited
Loading

I was testing this PR to double check this issue and found an odd UI issue, the size of the buttons on the "payment successful" screen when in landscape mode change as you go from portrait to landscape:

RPReplay_Final1673247511.MP4

@joshheald
8355 Consistent button size on orientation change
41d1e1f 
Previously, if you had the phone in landscape when the “Payment Successful” modal was shown, the two primary buttons would be displayed side by side at different sizes. Changing to portrait and back to landscape resulted in two different sizes showing, which felt strange.

Using the `fillProportionally` option resolves this and leads to a more consistent view in this edge case.
@joshheald
Copy link
Contributor Author

joshheald commented Jan 9, 2023

I was testing this PR to double check this issue and found an odd UI issue, the size of the buttons on the "payment successful" screen when in landscape mode change as you go from portrait to landscape:

Thanks @jostnes ... This is down to us changing the axis of the stack view so they can be side-by-side in landscape, then stacked in portrait. UIStackView is responsible for the size when it's rotated. Fortunately I was able to find a quick fix for this using the distribution property in 41d1e1f

rotation-buttons.mp4

@toupper
Copy link
Contributor

toupper commented Jan 9, 2023

@joshheald LGTM now, thanks for the changes.
As we talked, I saw that the loading indicator of the last alert (Getting ready to collect payment) overlaps other elements when switching the orientation exactly at that point. Unfortunately, it happened very fast so I couldn't create a screenshot, and now that I want to create a video I cannot rotate fast enough 😅 If I start the process on landscape mode it does not happen, as the loading indicator is hidden.
As this is a minor issue, feel free to prioritize it as you see appropriate.

@joshheald joshheald mentioned this pull request Jan 9, 2023
Open
@joshheald joshheald enabled auto-merge January 9, 2023 12:18
@joshheald joshheald merged commit 7bd2814 into trunk Jan 9, 2023
@joshheald joshheald deleted the issue/8355-update-tap-on-mobile-designs branch January 9, 2023 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@toupper toupper toupper approved these changes

@jostnes jostnes Awaiting requested review from jostnes

@lmischner lmischner Awaiting requested review from lmischner

Assignees

@toupper toupper

Labels

feature: mobile payments Related to mobile payments / card present payments / Woo Payments. status: feature-flagged Behind a feature flag. Milestone is not strongly held. type: enhancement A request for an enhancement.

Projects

None yet

Milestone

11.9

Development

Successfully merging this pull request may close these issues.

[Mobile Payments] Use new designs for Tap on Mobile connection and payment screens

6 participants

@joshheald @wpmobilebot @toupper @jostnes @malinajirka